feat[Go]: implement Box OAuth connector callback and poll-result APIs#15664
feat[Go]: implement Box OAuth connector callback and poll-result APIs#15664hunnyboy1217 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughAdds Box web OAuth: Redis-backed service state/result and token exchange; new ConnectorService methods (start/callback/poll); HTTP handlers for start, public callback, and polling; router entries and test stubs. ChangesBox OAuth Integration
sequenceDiagram
participant Browser as Browser (user agent)
participant CallbackHandler as ConnectorHandler.BoxWebOAuthCallback
participant ConnectorService as ConnectorService.BoxWebOAuthCallback
participant BoxToken as Box Token Endpoint
participant Redis as Redis
participant PollHandler as ConnectorHandler.PollBoxWebOAuthResult
Browser->>CallbackHandler: GET callback?state&code...
CallbackHandler->>ConnectorService: forward state,error,description,code
ConnectorService->>Redis: validate stored state
ConnectorService->>BoxToken: POST code exchange (exchangeBoxOAuthCode)
BoxToken-->>ConnectorService: tokens JSON
ConnectorService->>Redis: store result
PollHandler->>ConnectorService: poll flow_id
ConnectorService->>Redis: fetch & delete result
ConnectorService-->>PollHandler: return credentials
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi, @JinHai-CN, |
Hz-186
left a comment
There was a problem hiding this comment.
hi, @hunnyboy1217
You completely missed the implementation of the /start endpoint (POST /api/v1/connectors/box/oauth/web/start).
Without this endpoint, the frontend cannot initiate the OAuth flow, no flow_id is generated, and the initial state is never written to Redis. Calling this endpoint on the Go backend currently returns 404 Not Found.
Please add the missing implementation to achieve full parity with Python.
734881e to
2346685
Compare
Ports the full Box OAuth web flow from Python (connector_api.py) to Go, addressing issue infiniflow#15662 and the review on PR infiniflow#15664. POST /api/v1/connectors/box/oauth/web/start (StartBoxWebOAuth) GET /api/v1/connectors/box/oauth/web/callback (BoxWebOAuthCallback) POST /api/v1/connectors/box/oauth/web/result (PollBoxWebOAuthResult) Review fix (Hz-186): the /start endpoint was missing, so the frontend could not initiate the flow (no flow_id generated, no initial state written to Redis, 404 on the Go backend). Implemented StartBoxWebOAuth for full parity with Python start_box_web_oauth: - validates client_id / client_secret (ARGUMENT_ERROR when missing) - resolves redirect_uri (request value or BOX_WEB_OAUTH_REDIRECT_URI) - generates a flow_id, builds the Box authorize URL (https://account.box.com/api/oauth2/authorize?...&state=flow_id) - writes the initial boxWebOAuthState to Redis under the box flow-state key with the standard web-flow TTL - returns {flow_id, authorization_url, expires_in} Wired the authenticated POST route in the /api/v1/connectors group and added the service-interface method on the handler. Also addressed CodeRabbit's docstring-coverage warning: documented the Box request/response/state types, the new start + URL-builder helpers, and the previously-undocumented boxOAuthTokenResponse and defaultBoxWebOAuthRedirectURI.
|
Hi, @Hz-186, |
Ports the full Box OAuth web flow from Python (connector_api.py) to Go, addressing issue infiniflow#15662 and the review on PR infiniflow#15664. POST /api/v1/connectors/box/oauth/web/start (StartBoxWebOAuth) GET /api/v1/connectors/box/oauth/web/callback (BoxWebOAuthCallback) POST /api/v1/connectors/box/oauth/web/result (PollBoxWebOAuthResult) Review fix (Hz-186): the /start endpoint was missing, so the frontend could not initiate the flow (no flow_id generated, no initial state written to Redis, 404 on the Go backend). Implemented StartBoxWebOAuth for full parity with Python start_box_web_oauth: - validates client_id / client_secret (ARGUMENT_ERROR when missing) - resolves redirect_uri (request value or BOX_WEB_OAUTH_REDIRECT_URI) - generates a flow_id, builds the Box authorize URL (https://account.box.com/api/oauth2/authorize?...&state=flow_id) - writes the initial boxWebOAuthState to Redis under the box flow-state key with the standard web-flow TTL - returns {flow_id, authorization_url, expires_in} Wired the authenticated POST route in the /api/v1/connectors group and added the service-interface method on the handler. Also addressed CodeRabbit's docstring-coverage warning: documented the Box request/response/state types, the new start + URL-builder helpers, and the previously-undocumented boxOAuthTokenResponse and defaultBoxWebOAuthRedirectURI.
2346685 to
adf6047
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/service/connector.go`:
- Around line 993-999: boxWebOAuthState currently lacks a field for the
redirect_uri so when an OAuth flow is started with a custom redirect URI the
callback uses BOX_WEB_OAUTH_REDIRECT_URI instead, causing a mismatch during
token exchange; add a RedirectURI string field to boxWebOAuthState, populate it
wherever the authorize URL is built (the start flow that constructs the auth
URL), and then use that stored RedirectURI value during the token exchange
(instead of always falling back to BOX_WEB_OAUTH_REDIRECT_URI) so the same
redirect_uri is sent in both authorize and token requests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 05d3ca3a-0585-40b0-98b9-faa590845f17
📒 Files selected for processing (4)
internal/handler/connector.gointernal/handler/connector_test.gointernal/router/router.gointernal/service/connector.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/router/router.go
- internal/handler/connector.go
| type boxWebOAuthState struct { | ||
| UserID string `json:"user_id"` | ||
| AuthURL string `json:"auth_url"` | ||
| ClientID string `json:"client_id"` | ||
| ClientSecret string `json:"client_secret"` | ||
| CreatedAt int64 `json:"created_at"` | ||
| } |
There was a problem hiding this comment.
Persist the start-time redirect_uri and reuse it during token exchange.
If the caller supplies redirect_uri on start, Lines 1061-1068 build the authorize URL with that value, but boxWebOAuthState never stores it. The callback then drops back to BOX_WEB_OAUTH_REDIRECT_URI on Line 1201, so the token request can send a different redirect_uri than the authorization request. That breaks any flow that relies on a non-default redirect URI.
Suggested fix
type boxWebOAuthState struct {
UserID string `json:"user_id"`
AuthURL string `json:"auth_url"`
ClientID string `json:"client_id"`
ClientSecret string `json:"client_secret"`
+ RedirectURI string `json:"redirect_uri,omitempty"`
CreatedAt int64 `json:"created_at"`
}
@@
state := boxWebOAuthState{
UserID: userID,
AuthURL: authURL,
ClientID: clientID,
ClientSecret: clientSecret,
+ RedirectURI: redirectURI,
CreatedAt: time.Now().Unix(),
}
@@
- accessToken, refreshToken, err := exchangeBoxOAuthCode(state.ClientID, state.ClientSecret, code)
+ accessToken, refreshToken, err := exchangeBoxOAuthCode(state.ClientID, state.ClientSecret, state.RedirectURI, code)
@@
-func exchangeBoxOAuthCode(clientID, clientSecret, code string) (accessToken, refreshToken string, err error) {
+func exchangeBoxOAuthCode(clientID, clientSecret, redirectURI, code string) (accessToken, refreshToken string, err error) {
@@
- redirectURI := defaultBoxWebOAuthRedirectURI()
if redirectURI != "" {
form.Set("redirect_uri", redirectURI)
}Also applies to: 1061-1080, 1143-1146, 1201-1204
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/service/connector.go` around lines 993 - 999, boxWebOAuthState
currently lacks a field for the redirect_uri so when an OAuth flow is started
with a custom redirect URI the callback uses BOX_WEB_OAUTH_REDIRECT_URI instead,
causing a mismatch during token exchange; add a RedirectURI string field to
boxWebOAuthState, populate it wherever the authorize URL is built (the start
flow that constructs the auth URL), and then use that stored RedirectURI value
during the token exchange (instead of always falling back to
BOX_WEB_OAUTH_REDIRECT_URI) so the same redirect_uri is sent in both authorize
and token requests.
Closes #15662.
GET /api/v1/connectors/box/oauth/web/callback (BoxWebOAuthCallback)
POST /api/v1/connectors/box/oauth/web/result (PollBoxWebOAuthResult)
The callback endpoint is public (registered without auth middleware, matching the Google/Gmail pattern). Box redirects the user's browser here after consent; the handler reads the state from Redis, exchanges the authorization code for an access + refresh token pair via POST https://api.box.com/oauth2/token, stores the result in Redis, and renders an HTML self-closing popup page.
The result endpoint requires authentication. It retrieves the stored credential set, verifies the caller owns the flow (user_id match), deletes the Redis key, and returns the full credential payload — mirroring Python poll_box_web_result.
Redis key scheme reuses the existing helpers:
state: box_web_flow_state:{flow_id}
result: box_web_flow_result:{flow_id}
BOX_WEB_OAUTH_REDIRECT_URI env var is forwarded to the token exchange when set; omitted otherwise (Box accepts the absence when the redirect_uri was optional).
Changed files:
internal/service/connector.go – Box types + BoxWebOAuthCallback +
PollBoxWebOAuthResult + exchangeBoxOAuthCode
internal/handler/connector.go – interface extension + two new handlers
internal/router/router.go – callback registered at engine + apiNoAuth;
result registered in auth-protected connector group
What problem does this PR solve?
Briefly describe what this PR aims to solve. Include background context that will help reviewers understand the purpose of the PR.
Type of change